packages/project-builder-server/src/actions/definition/discard-draft.action.ts (1)

8-10: Consider adding minimum length validation for the project input.

Using z.string().min(1) would reject empty strings at schema validation, providing a clearer error message to the user rather than failing downstream in getProjectByNameOrId.

💡 Suggested improvement

 const discardDraftInputSchema = z.object({
-  project: z.string().describe('The name or ID of the project.'),
+  project: z.string().min(1).describe('The name or ID of the project.'),
 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/project-builder-server/src/actions/definition/discard-draft.action.ts`
around lines 8 - 10, Update the Zod schema for discardDraftInputSchema to reject
empty strings by adding a minimum length check: change the project field from
z.string() to z.string().min(1) (and optionally include a .describe() or
.message()) so validation fails fast and clearer errors are returned before
reaching getProjectByNameOrId; ensure the reference name discardDraftInputSchema
is used and tests/consumers expecting validation messages are adjusted if
needed.

packages/project-builder-server/src/actions/definition/draft-session.ts (1)

62-68: Consider resilience for partially corrupted draft state.

If draft-session.json exists but draft-definition.json is missing (e.g., due to a previous write failure), loadDraftSession will throw an unhandled error from readFile. Consider checking both files exist before reading, or catching the error to treat it as a corrupted draft that should be discarded.

🛠️ Suggested fix

 export async function loadDraftSession(
   projectDirectory: string,
 ): Promise<DraftSession | null> {
   const sessionPath = getDraftSessionPath(projectDirectory);
-  if (!(await fileExists(sessionPath))) {
+  const definitionPath = getDraftDefinitionPath(projectDirectory);
+  if (!(await fileExists(sessionPath)) || !(await fileExists(definitionPath))) {
     return null;
   }
   const [sessionContents, definitionContents] = await Promise.all([
     readFile(sessionPath, 'utf-8'),
-    readFile(getDraftDefinitionPath(projectDirectory), 'utf-8'),
+    readFile(definitionPath, 'utf-8'),
   ]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/project-builder-server/src/actions/definition/draft-session.ts`
around lines 62 - 68, loadDraftSession currently checks only draft-session.json
existence then calls readFile on both files; if draft-definition.json is missing
a readFile will throw. Modify loadDraftSession to either 1) check existence of
both files (use fileExists on sessionPath and on
getDraftDefinitionPath(projectDirectory)) and return null if either is missing
before calling readFile, or 2) wrap the Promise.all([...readFile...]) in a
try/catch and treat any read/parse error as a corrupted draft (return null and
optionally remove the partial files); reference loadDraftSession, sessionPath,
getDraftDefinitionPath, fileExists, and readFile when making the change.

packages/project-builder-server/src/actions/definition/stage-create-entity.action.ts (1)

28-34: Return the generated entity id from stage-create-entity.

This action stages a newly generated entity but only returns a generic message. Exposing the created entityId here would make follow-up stage-update-entity / stage-delete-entity calls against draft-only entities scriptable without forcing callers to inspect show-draft.

Also applies to: 47-54, 70-73

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/project-builder-server/src/actions/definition/stage-create-entity.action.ts`
around lines 28 - 34, Update the staged-entity action to return the created
entity's id: add an entityId field (string, e.g. z.string().describe('ID of the
staged entity')) to stageCreateEntityOutputSchema and ensure the action handler
that calls the entity creation returns { message, entityId, issues? } (use the
actual id returned by the create/storage call inside the function that performs
staging, e.g., where createEntity/insertDraftEntity is invoked). Also update any
other places that construct this action's response (the code paths referenced
around the other return blocks) so they include the same entityId value or
null/undefined consistently when no id was created. Ensure all references use
the exact symbol stageCreateEntityOutputSchema and the action handler that
builds the response so the schema and runtime result stay in sync.

packages/project-builder-server/src/actions/definition/get-entity-schema.action.ts (1)

45-48: Point the recovery hint at list-entity-types.

This PR already adds a dedicated discovery action, so using it here keeps the recovery path aligned with the actual API instead of depending on wildcard behavior in list-entities.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/project-builder-server/src/actions/definition/get-entity-schema.action.ts`
around lines 45 - 48, The error message thrown in get-entity-schema.action.ts
when metadata is missing references the wrong recovery hint; update the thrown
Error where it checks if (!metadata) to suggest using the new discovery action
name "list-entity-types" instead of "list-entities" with wildcard. Locate the
throw that builds the message with input.entityTypeName and replace the recovery
hint text to point developers to use list-entity-types for discovering available
types while keeping the existing descriptive context.

packages/project-builder-server/src/actions/definition/definition-actions.int.test.ts (2)

30-31: Mock node:fs alongside node:fs/promises.

This suite is exercising draft-file persistence through memfs. Leaving the sync fs module live makes it easy for future implementation changes to bypass the virtual filesystem and hit the host filesystem instead.

📦 Small test-hardening diff

 vi.mock('./load-entity-service-context.js');
+vi.mock('node:fs');
 vi.mock('node:fs/promises');

Based on learnings, in the project-builder-server test suite, Vitest automocks for `node:fs` and `node:fs/promises` are already configured to use memfs without needing explicit implementation replacement in each test file.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/project-builder-server/src/actions/definition/definition-actions.int.test.ts`
around lines 30 - 31, The test currently mocks
'./load-entity-service-context.js' and 'node:fs/promises' but leaves the
synchronous 'node:fs' module live, which can bypass memfs; add a mock for
'node:fs' (e.g., call vi.mock('node:fs')) alongside the existing
vi.mock('node:fs/promises') in the same file so all fs APIs used by the test run
against the virtual filesystem; update the test header where
vi.mock('./load-entity-service-context.js') and vi.mock('node:fs/promises') are
declared to include vi.mock('node:fs').

---

215-304: Add one end-to-end draft lifecycle test.

This suite proves the stage-* writes, but the new public workflow also depends on show-draft, commit-draft, and discard-draft. One happy-path test through that flow would catch regressions in the .build/draft-*.json contract.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/project-builder-server/src/actions/definition/definition-actions.int.test.ts`
around lines 215 - 304, Add a single end-to-end test that exercises the full
draft lifecycle: call stageCreateEntityAction (or reuse existing
stage-create-entity test setup) then invoke the show-draft action
(showDraftAction or the "show-draft" CLI action) to assert the draft files
contain the staged change, call commit-draft (commitDraftAction /
"commit-draft") and assert the committed definition on disk includes the new
feature and that draft-*.json files are removed/cleared, and finally call
discard-draft (discardDraftAction / "discard-draft") or assert no leftover
drafts; reference the existing test helpers invokeServiceActionAsCli,
PROJECT_DIR/.build/draft-session.json and draft-definition.json, and the entity
name 'payments' to locate where to add this new end-to-end test.

packages/project-builder-lib/src/tools/entity-service/entity-write.ts (2)

85-91: Validation check doesn't use the retrieved value.

The code retrieves target and validates it's a plain object, but then replaces it entirely with updatedEntity at line 108. The validation is still useful as a defensive check, but consider adding a comment to clarify the intent, or removing the unused isPlainObject check since you're replacing the entire object anyway.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/project-builder-lib/src/tools/entity-service/entity-write.ts` around
lines 85 - 91, The isPlainObject check currently validates the value returned by
get(draft, entity.path) but that retrieved value is never used because you later
replace the node with updatedEntity; either remove the redundant isPlainObject
check or convert it into an explicit defensive assertion/comment that clarifies
intent (e.g., "ensure existing node is an object before replacing") and keep it
if you want to guard against replacing non-object paths; reference the
get(draft, entity.path) call, the isPlainObject check, and the subsequent
replacement with updatedEntity and update the code comment or remove the check
accordingly so the intent matches the behavior.

---

93-100: Consider extracting shared path resolution logic.

Both updateEntity and deleteEntity contain identical logic to extract parentPath and entityIndex from entity.path. This could be extracted into a small helper function for clarity and DRY.

♻️ Optional refactor

function extractParentArrayInfo(entityPath: (string | number)[]): {
  parentPath: (string | number)[];
  entityIndex: number;
} {
  const parentPath = entityPath.slice(0, -1);
  const entityIndex = entityPath.at(-1);
  if (typeof entityIndex !== 'number') {
    throw new TypeError(
      `Expected numeric index at end of entity path "${entityPath.join('.')}"`,
    );
  }
  return { parentPath, entityIndex };
}

Also applies to: 139-146

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/project-builder-lib/src/tools/entity-service/entity-write.ts` around
lines 93 - 100, Extract the duplicated path-resolution logic in updateEntity and
deleteEntity into a small helper like extractParentArrayInfo that accepts
entity.path (or entityPath: (string|number)[]) and returns { parentPath,
entityIndex: number }; inside it call slice(0,-1), use .at(-1) and throw the
existing TypeError if entityIndex is not a number, then replace the inlined
logic in both updateEntity and deleteEntity to call this helper and use its
returned parentPath and entityIndex values to keep behavior identical and DRY.

packages/project-builder-server/src/actions/definition/commit-draft.action.ts (1)

25-31: Output schema issues field is never populated.

The commitDraftOutputSchema declares an optional issues array (lines 27-30), but when validation issues occur, they are thrown as an error (lines 79-84) rather than returned. The successful return (lines 96-98) never includes issues. Either remove the unused field from the schema or return issues as data when appropriate.

🔧 Proposed fix: remove unused field

 const commitDraftOutputSchema = z.object({
   message: z.string().describe('A summary of the commit result.'),
-  issues: z
-    .array(definitionIssueSchema)
-    .optional()
-    .describe('Definition issues that blocked the commit.'),
 });

Also applies to: 79-84, 96-98

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/project-builder-server/src/actions/definition/commit-draft.action.ts`
around lines 25 - 31, The output schema commitDraftOutputSchema declares an
optional issues array but the commit-draft logic currently throws validation
errors instead of returning them and the success return never includes issues;
update the commitDraft action so that when validation or definition issues are
detected (where errors are currently thrown around the validation block) you
transform those errors into an array matching definitionIssueSchema and return
them as the issues property on the successful response object (e.g., return {
message: string, issues }), or alternatively remove the unused issues field from
commitDraftOutputSchema if you prefer not to return issues; locate the throw
path (currently lines around the validation/error handling) and the successful
return (currently the success return block) and modify one of those spots to
either populate issues or delete the schema field accordingly.

packages/project-builder-lib/src/tools/entity-service/entity-read.ts (2)

67-76: JSDoc parameter documentation is also outdated for getEntity.

Same issue as above - the documented parameters don't match the actual function signature.

📝 Proposed fix

 /**
  * Gets a single entity by ID, returning its full serialized (name-based) data.
  *
- * `@param` container - The project definition container
- * `@param` entityTypeMap - The entity type map built from the schema
- * `@param` serializedDef - The serialized project definition (with names)
- * `@param` entityTypeName - The entity type to get
- * `@param` entityId - The entity ID
+ * `@param` entityId - The entity ID to look up
+ * `@param` context - The entity service context
  * `@returns` The serialized entity data, or undefined if not found
  */

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/project-builder-lib/src/tools/entity-service/entity-read.ts` around
lines 67 - 76, The JSDoc for getEntity is out of sync with its actual
signature—update the comment block above the getEntity function to exactly match
the real parameter names and return type used in the implementation (e.g.,
ensure parameters listed match the function's signature such as container,
entityTypeMap, serializedDef, entityTypeName, entityId or whatever the current
parameter names are in getEntity) and adjust the `@returns` description to reflect
the actual returned type (serialized entity or undefined); keep descriptions
brief and accurate and remove any stale params that no longer exist.

---

23-34: JSDoc parameter documentation is outdated.

The JSDoc lists individual parameters (container, entityTypeMap, serializedDef, etc.) but the actual function signature uses a destructured input object and a context parameter. Update the documentation to match the actual API.

📝 Proposed fix

 /**
  * Lists all entities of a given type, returning lightweight stubs.
  *
  * For nested entity types, `parentEntityId` is required to scope the listing
  * to entities within a specific parent.
  *
- * `@param` container - The project definition container
- * `@param` entityTypeMap - The entity type map built from the schema
- * `@param` entityTypeName - The entity type to list
- * `@param` parentEntityId - Required for nested entity types
+ * `@param` input - The input containing entityTypeName and optional parentEntityId
+ * `@param` context - The entity service context
  * `@returns` Array of entity stubs with id, name, and type
  */

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/project-builder-lib/src/tools/entity-service/entity-read.ts` around
lines 23 - 34, The JSDoc is out of sync with the actual function signature in
entity-read.ts: update the comment to document the destructured input object and
the separate context parameter instead of individual params like `container` and
`entityTypeMap`; replace the `@param` entries with a single `@param` input that
lists the destructured fields (e.g. container, entityTypeMap, serializedDef,
entityTypeName, parentEntityId) and add `@param` context describing its
shape/purpose, and ensure the `@returns` description matches the actual return
value of the function (entity stubs with id, name, type). Reference the function
implemented in entity-read.ts and adjust the JSDoc tags to match its
destructured input and context parameters.